I've started the review, and I have one question regarding the database seeding. How do you plan to update the database on the VM? Doing pnpm db:setup will delete the old database, and then create and seed the new one, which will make current users lose their data.

I'm thinking about how to properly seed new fields and data into the existing database without changing what's currently there, but I wanted to check with you if you have already thought about this. If not, I can look into a solution on how to build a script that properly seeds a database without altering existing values. Later I want to include ratings as well, so this is probably something we will encounter more times.

Given that we don't really have any real users, it shouldn't be a problem to re-create the database. If we want to keep the data, we can create migrations using Drizzle, and this is already set up - however, that only creates SQL code that converts the database from the old version to the new one, but the data is still kept as it was. Given how much code this would require, and that we will probably not change the seeding script too many times again, I do not think it's a good time investment to build additional scripts that make it so we can keep the very little user data we have generated.

Of course, we should probably make this a group decision and discuss it further

Merged main into this branch, so the diff is much clearer and the review should be much easier to do

Given that we don't really have any real users, it shouldn't be a problem to re-create the database. If we want to keep the data, we can create migrations using Drizzle, and this is already set up - however, that only creates SQL code that converts the database from the old version to the new one, but the data is still kept as it was. Given how much code this would require, and that we will probably not change the seeding script too many times again, I do not think it's a good time investment to build additional scripts that make it so we can keep the very little user data we have generated.

Of course, we should probably make this a group decision and discuss it further

Agreed that it should be a group decision. In terms of sustainability and scalabilty it would be nice to show that we can update the database in the most cost efficient way. I get that this is not a big database, and re-creating this is a small operation, but since the requirements looks says that the database should support a large amount of data ( i don't remember the wordings exactly), I still think having some way of updating the database is the most future-proof path.

1. Choosing filter on a page doesn't reset the page, meaning that if I'm on page 10, and then choosing a narrow filter, it will just say "No books found". I changed the url to go to page 1, and then it showed the results.

2. Raw SQL error message is shown.

3. The "sjanger" filter is working fine. The only thing I want us to discuss regarding the filter is if we should give the user feedback that it is a "OR" filter, and a "AND" filter. Also an additional question to that is that if it should be a "AND" filter. Thoughts? I noticed that you have made a conscious decision on this in this commit (0a5927d) so I think we can leave it at that.

4. Might be just poor user behaviour by myself, but the Pages slider was giving me a hard time. It worked fine, but I thought it was number of pages in the application, not number of pages in a book. I think a small re-wording makes sense her. I'll bring you a suggestion.

I do think it is mergable, and best that these comments becomes separate smaller issues so other people can work on this in parallell.

Good work!

Following up on what I mentioned in person.

At first, I thought I had resolved the first comment (1), but after taking a closer look, it seems like I haven't.

In 78c285b I added a fix to reset to the first page when the search changes, and in 46103d9 I made sure that pressing the "Reset all filters" button also resets the page to page 1. Both are part of #56 (slightly unrelated to what #56 is about, but anyways).

So it looks like the specific issue that is raised here, where changing a filter doesn't reset the page, is still unresolved.

Resetting the page search param on changing filters: Fixed in 9d2f92b
Improving error messages: Own issue, see #58
Genre filter: I think it makes the most sense to leave it as OR, given that few books have multiple genres attached to them. Using AND just generally makes the search very specific and leaves us with very few books
Change name of "pages" filter: Changed to "Book length (pages)", as you suggested, in 491e5f0

Since #56 changes the look of the book filters, this should probably be merged after that PR. This way I'll adjust the filters so they look like the redesign done in #56

This PR is now synced with the design change of the filters from #56.
We now have a Popover with Selects inside, which makes the user see two "dropdown" stacked on each other. The UX (user experience) of this is debatable. I would suggest discussing whether we should move this to a "flat" menu, such as a side bar or similar that slides in from the side of the screen.

This PR is now synced with the design change of the filters from #56. We now have a Popover with Selects inside, which makes the user see two "dropdown" stacked on each other. The UX (user experience) of this is debatable. I would suggest discussing whether we should move this to a "flat" menu, such as a side bar or similar that slides in from the side of the screen.

Yes, we can discuss this, but I'm approving, and we can deal with that later.